Extend optimization methods #37
Conversation
|
PR brakes backwards compatibility because I have renamed the branchlength_optim! kwarg bl_optimizer to bl_modifer and the nni_optim! kwarg acc_rule to nni_selection_rule, (it is because of this the runtest.jl fails, it is a instance where we call bl_optimizer=BrentsMethodOpt() instead of bl_modifer = BrentsMethodOpt()) |
There was a problem hiding this comment.
Looks good in general. I think we should break backwards-compat since this PR allows for more flexible behaviours of branchlength_optim! and nni_optim!. kwargs should probably be renamed for the sake of clarity.
See my more detailed comments and please update the tests so that they pass.
|
|
||
| Returns a untangled copy of the a tree. Optionally, the flag `shallow_copy` can be used to obtained a copy of the tree with only the names and branchlengths. | ||
| """ | ||
| function copy_tree(root::FelNode, shallow_copy=false) |
There was a problem hiding this comment.
do you have any performance comparisons between deepcopy(tree) and copy_tree(tree)?
There was a problem hiding this comment.
I haven't benchmarked this. I can look into this. Do you have any specific benchmark in mind? Like the time etc.?
There was a problem hiding this comment.
you could use @benchmark from BenchmarkTools.jl to get time, allocs etc.
Added functionality to sample tree topologies with the metropolis_sample function, given a branch model and leaf node data.